security: remove sensitive data from logs and escape HTML in tx details - #4677
security: remove sensitive data from logs and escape HTML in tx details#4677Ismail-Benali wants to merge 2 commits into
Conversation
Remove console.log statements that printed transaction IDs, wallet addresses, payment IDs and payment-proof signatures to the debug log. Escape all user/chain-supplied fields in buildTxDetailsString before inserting them into the rich-text table shown in the transaction details dialog, preserving the intentional <br> line separators in the destinations field.
91c474d to
b712912
Compare
|
Not 100% sure (worth checking), but i think active wallet primary address is logged on the default lvl (from wallet2) when you open the wallet, so removing when you close the wallet wouldnt help there. the logging of the address is also normal in other programs, such as monero-wallet-rpc |
|
Thanks for the review @jpk68 and @nahuhh. @jpk68: you're right — I had missed those two spots. I've now removed the console.log calls in handleGetProof and handleCheckProof (txid, address, message and the payment-proof signature) in commit 5efbc6d, so the same class of sensitive data is no longer written to the logs there either. @nahuhh: good point about wallet2 logging the primary address at the default level on open. That logging lives in the monero-core submodule (wallet2.cpp), which is outside this repository, so it can't be changed in this PR. Our changes here only remove the additional GUI-side logging of addresses/txids/signatures on top of what the core already emits. |
3 participants
Summary
Two low-severity security hardening changes in the QML UI.
1. Stop logging sensitive wallet data
Several
console.logcalls wrote transaction IDs, wallet addresses, payment IDs and payment-proof signatures to the debug output:pages/History.qml: proof generation logged txid + addresspages/TxKey.qml: proof generation and verification logged txid + address + signaturepages/AddressBook.qml: sending logged address + paymentIdmain.qml: failed wallet open logged wallet.addressA payment-proof signature is a signed proof of ownership; writing it to the log alongside the txid weakens the proof's usefulness if the log ever leaks.
2. Escape HTML in the transaction details dialog
buildTxDetailsStringinpages/History.qmlbuilds a rich-text table (StandardDialogusesTextEdit.AutoText) with values that originate from the user or the chain. Only tx_note was escaped; the remaining fields (tx_id, address, paymentId, integratedAddress, tx_key, rings, destinations) were inserted raw. All fields are now passed throughUtils.htmlEscape(). The intentional<br>line separators inside the destinations field are restored after escaping so the display is unchanged.Files changed
Testing
Not built; changes are limited to removing log lines and HTML-escaping string interpolation. No functional behavior is intended to change.